[RISCV] Mark More Relocs as Relaxable #151422
Conversation
Since this code was last reviewed, more relaxations have been added for existing standard relocations that LLVM didn't have marked as relaxable. This code ensures that LLVM marks the following relocations (and their respective fixups) as relaxable: - `R_RISCV_JAL` - `R_RISCV_GOT_HI20` - `R_RISCV_TPREL_HI20` - `R_RISCV_TLSDESC_HI20`
|
@llvm/pr-subscribers-mc @llvm/pr-subscribers-backend-risc-v Author: Sam Elliott (lenary) ChangesSince this code was last reviewed, more relaxations have been added to the psABI for existing standard relocations that LLVM didn't have marked as relaxable. This change ensures that LLVM marks the following relocations (and their respective fixups) as relaxable:
This also updates the linker relaxation test to use Full diff: https://github.com/llvm/llvm-project/pull/151422.diff 2 Files Affected:
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
index cbeabdddb9371..c7135c8e7ab7f 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
@@ -628,9 +628,6 @@ uint64_t RISCVMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNo,
llvm_unreachable("VK_TPREL_LO used with unexpected instruction format");
RelaxCandidate = true;
break;
- case ELF::R_RISCV_TPREL_HI20:
- RelaxCandidate = true;
- break;
case ELF::R_RISCV_CALL_PLT:
FixupKind = RISCV::fixup_riscv_call_plt;
RelaxCandidate = true;
@@ -639,11 +636,17 @@ uint64_t RISCVMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNo,
FixupKind = RISCV::fixup_riscv_qc_abs20_u;
RelaxCandidate = true;
break;
+ case ELF::R_RISCV_GOT_HI20:
+ case ELF::R_RISCV_TPREL_HI20:
+ case ELF::R_RISCV_TLSDESC_HI20:
+ RelaxCandidate = true;
+ break;
}
} else if (Kind == MCExpr::SymbolRef || Kind == MCExpr::Binary) {
// FIXME: Sub kind binary exprs have chance of underflow.
if (MIFrm == RISCVII::InstFormatJ) {
FixupKind = RISCV::fixup_riscv_jal;
+ RelaxCandidate = true;
} else if (MIFrm == RISCVII::InstFormatB) {
FixupKind = RISCV::fixup_riscv_branch;
} else if (MIFrm == RISCVII::InstFormatCJ) {
diff --git a/llvm/test/MC/RISCV/linker-relaxation.s b/llvm/test/MC/RISCV/linker-relaxation.s
index 6b0685baaa69e..c5c4e4877ff2e 100644
--- a/llvm/test/MC/RISCV/linker-relaxation.s
+++ b/llvm/test/MC/RISCV/linker-relaxation.s
@@ -8,49 +8,57 @@
# RUN: | llvm-readobj -r - | FileCheck -check-prefix=NORELAX-RELOC %s
.long foo
+# NORELAX-RELOC: R_RISCV_32 foo 0x0
+# RELAX-RELOC: R_RISCV_32 foo 0x0
call foo
-# NORELAX-RELOC: R_RISCV_CALL_PLT foo 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_CALL_PLT foo 0x0
# NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_CALL_PLT foo 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_CALL_PLT foo 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
+
+jal foo
+# NORELAX-RELOC-NEXT: R_RISCV_JAL foo 0x0
+# NORELAX-RELOC-NOT: R_RISCV_RELAX
+# RELAX-RELOC-NEXT: R_RISCV_JAL foo 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
lui t1, %hi(foo)
-# NORELAX-RELOC: R_RISCV_HI20 foo 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_HI20 foo 0x0
# NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_HI20 foo 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_HI20 foo 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
addi t1, t1, %lo(foo)
-# NORELAX-RELOC: R_RISCV_LO12_I foo 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_LO12_I foo 0x0
# NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_LO12_I foo 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_LO12_I foo 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
sb t1, %lo(foo)(a2)
-# NORELAX-RELOC: R_RISCV_LO12_S foo 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_LO12_S foo 0x0
# NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_LO12_S foo 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_LO12_S foo 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
1:
auipc t1, %pcrel_hi(foo)
-# NORELAX-RELOC: R_RISCV_PCREL_HI20 foo 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_PCREL_HI20 foo 0x0
# NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_PCREL_HI20 foo 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_PCREL_HI20 foo 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
addi t1, t1, %pcrel_lo(1b)
-# NORELAX-RELOC: R_RISCV_PCREL_LO12_I .Ltmp0 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_PCREL_LO12_I .Ltmp0 0x0
# NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_PCREL_LO12_I .Ltmp0 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_PCREL_LO12_I .Ltmp0 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
sb t1, %pcrel_lo(1b)(a2)
-# NORELAX-RELOC: R_RISCV_PCREL_LO12_S .Ltmp0 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_PCREL_LO12_S .Ltmp0 0x0
# NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_PCREL_LO12_S .Ltmp0 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_PCREL_LO12_S .Ltmp0 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
# Check behaviour when a locally defined symbol is referenced.
@@ -63,64 +71,88 @@ call bar
# NORELAX-RELOC-NOT: R_RISCV_CALL
# NORELAX-RELOC-NOT: R_RISCV_RELAX
# RELAX-RELOC-NEXT: R_RISCV_CALL_PLT bar 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
+
+jal bar
+# NORELAX-RELOC-NOT: R_RISCV_JAL
+# NORELAX-RELOC-NOT: R_RISCV_RELAX
+# RELAX-RELOC-NEXT: R_RISCV_JAL bar 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
beq s1, s1, bar
# NORELAX-RELOC-NOT: R_RISCV_BRANCH
# RELAX-RELOC-NEXT: R_RISCV_BRANCH bar 0x0
lui t1, %hi(bar)
-# NORELAX-RELOC: R_RISCV_HI20 bar 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_HI20 bar 0x0
# NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_HI20 bar 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_HI20 bar 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
addi t1, t1, %lo(bar)
-# NORELAX-RELOC: R_RISCV_LO12_I bar 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_LO12_I bar 0x0
# NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_LO12_I bar 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_LO12_I bar 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
sb t1, %lo(bar)(a2)
-# NORELAX-RELOC: R_RISCV_LO12_S bar 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_LO12_S bar 0x0
# NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_LO12_S bar 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_LO12_S bar 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
2:
auipc t1, %pcrel_hi(bar)
# NORELAX-RELOC-NOT: R_RISCV_PCREL_HI20
# NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_PCREL_HI20 bar 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_PCREL_HI20 bar 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
addi t1, t1, %pcrel_lo(2b)
# NORELAX-RELOC-NOT: R_RISCV_PCREL_LO12_I
# NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_PCREL_LO12_I .Ltmp1 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_PCREL_LO12_I .Ltmp1 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
sb t1, %pcrel_lo(2b)(a2)
# NORELAX-RELOC-NOT: R_RISCV_PCREL_LO12_S
# NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_PCREL_LO12_S .Ltmp1 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_PCREL_LO12_S .Ltmp1 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
+
+auipc t1, %got_pcrel_hi(bar)
+# NORELAX-RELOC-NEXT: R_RISCV_GOT_HI20 bar 0x0
+# NORELAX-RELOC-NOT: R_RISCV_RELAX
+# RELAX-RELOC-NEXT: R_RISCV_GOT_HI20 bar 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
+
+lui t1, %tprel_hi(baz)
+# NORELAX-RELOC-NEXT: R_RISCV_TPREL_HI20 baz 0x0
+# NORELAX-RELOC-NOT: R_RISCV_RELAX
+# RELAX-RELOC-NEXT: R_RISCV_TPREL_HI20 baz 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
+
+auipc t1, %tlsdesc_hi(baz)
+# NORELAX-RELOC-NEXT: R_RISCV_TLSDESC_HI20 baz 0x0
+# NORELAX-RELOC-NOT: R_RISCV_RELAX
+# RELAX-RELOC-NEXT: R_RISCV_TLSDESC_HI20 baz 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
## %hi/%lo on an absolute symbol (not yet defined) leads to relocations when relaxation is enabled.
lui t2, %hi(abs)
# NORELAX-RELOC-NOT: R_RISCV_
-# RELAX-RELOC: R_RISCV_HI20 - 0x12345
+# RELAX-RELOC-NEXT: R_RISCV_HI20 - 0x12345
# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
addi t2, t2, %lo(abs)
# NORELAX-RELOC-NOT: R_RISCV_
-# RELAX-RELOC: R_RISCV_LO12_I - 0x12345
+# RELAX-RELOC-NEXT: R_RISCV_LO12_I - 0x12345
# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
.set abs, 0x12345
lui t3, %hi(abs)
-# RELAX-RELOC: R_RISCV_HI20 - 0x12345
+# RELAX-RELOC-NEXT: R_RISCV_HI20 - 0x12345
# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
# Check that a relocation is not emitted for a symbol difference which has
|
| ; CHECK-NEXT: auipc ra, 0x0 | ||
| ; CHECK-NEXT: R_RISCV_CALL_PLT f | ||
| ; RELAX-NEXT: R_RISCV_RELAX *ABS* | ||
| ; CHECK-NEXT: jalr ra | ||
| ; CHECK-NEXT: j {{.*}} | ||
| ; RELAX-NEXT: R_RISCV_JAL {{.*}} | ||
| ; RELAX-NOT: R_RISCV_RELAX |
There was a problem hiding this comment.
I believe J is not supposed to be assembler- or linker- relaxable. Then we should not mark it as such.
There was a problem hiding this comment.
The Table Jump Relaxation applies to JAL and want the R_RISCV_JAL to be marked with R_RISCV_RELAX: https://riscv-non-isa.github.io/riscv-elf-psabi-doc/#_table_jump_relaxation
There was a problem hiding this comment.
Even aside from that couldn't jal be relaxed to c.jal and j to c.j (with the former only being available on RV32)? I don't think that's a currently-implemented relaxation, it's only done if the original (pseudo)instruction is a call, but for those that believe in the fun that is linker relaxation it would seem an obvious one to implement.
|
I would prevent JAL from being assembler-relaxed to QC_E_JAL, resolving the issue. There is no interference for users who don't use the extension. Instead, the compiler should emit qc.e.jal when the displacement might exceed JAL's range, and the linker should handles QC_E_JAL->JAL accordingly. |
I don't want to side-track this patch in discussions about Xqci, which is why this patch does not reference QC.E.JAL or Xqci at all. We can come back to those issues on relevant patches/issues, there are enough of them floating around. The psABI specifically notes relaxations involving the relocations (or their respective fixups) updated in this patch. Yes, this will expose a layout problem in the backend (#150071). That's part of the reason I want to get this landed now, so we have time to fix it before a release. This patch is not against the 21.x branch and I do not intend to backport it. |
| @@ -1,32 +1,45 @@ | |||
| ;; With +relax, J below needs a relocation to ensure the target is correct | |||
| ;; after linker relaxation. See https://github.com/ClangBuiltLinux/linux/issues/1965 | |||
|
|
|||
|
This needs quite a lot of updates, due to #152602 and riscv-non-isa/riscv-elf-psabi-doc#475 - I will get to that in the next few days, but not today. |
Since this code was last reviewed, more relaxations have been added to the psABI for existing standard relocations that LLVM didn't have marked as relaxable.
This change ensures that LLVM marks the following relocations (and their respective fixups) as relaxable:
R_RISCV_JALR_RISCV_GOT_HI20R_RISCV_TPREL_HI20R_RISCV_TLSDESC_HI20This also updates the linker relaxation test to use
-NEXTto check all the output lines.